Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed some undefined indexes and other errors setting up Google #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haroldkyle
Copy link

These problems could exist in other login files, but I was just looking at Google. Don't want to start the session if it's already started. Other undefined indexes fixed.

@perrybutler
Copy link
Owner

Looks good, will finish reviewing and merge this weekend. I don't think there was a major concern with calling session_start() blindly or twice (other than an E_NOTICE error), but your fix appears to be a more standard pattern which couldn't hurt to implement. I'll also need to check if session_start() is actually being called twice and throwing an error somewhere, since I'm curious about that now.

Some refs:

http://stackoverflow.com/questions/2580322/is-there-any-harm-in-running-session-start-multiple-times-as-the-page-request

http://php.net/manual/en/function.session-status.php

@haroldkyle
Copy link
Author

Yup, just trying to quiet my logs. Thanks!

Good point on checking whether this is called elsewhere in this plugin. I grepped my wp-content directory and didn't find many other possibilities, so it may well be this plugin trying to start the session multiple times.

@jazbek
Copy link

jazbek commented Apr 23, 2015

Would be great if you could merge this in and release it, lots of php notices are being thrown by this plugin when WP_DEBUG is on. This pull request fixes a big one that is on the main login form (wp-login.php).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants